Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve examples and README #102

Merged
merged 21 commits into from
Nov 17, 2023
Merged

Improve examples and README #102

merged 21 commits into from
Nov 17, 2023

Conversation

temeddix
Copy link
Contributor

@temeddix temeddix commented Nov 14, 2023 •
edited
Loading

This PR improves the example from various aspects.

  • Organize imports and variable names with the Ruff formatter.
  • Fix aiohttp.ClientSession() error saying that it should be run in an async function.
  • Upgrade to PySide6 and PyQt6.
  • Use asyncio.Event instead of CancelledError exception to close the app gracefully.
  • Use PySide6.QApplication. This is important because this class allows various font, theme, behavior setup. With QApplication.instance, app customization is not possible.
  • Remove unnecessary details

And of course, I confirmed that this code works properly.

@temeddix temeddix changed the title Update and organize example Update and organize examples Nov 14, 2023
hosaka
hosaka reviewed Nov 15, 2023
View reviewed changes
tests/test_qeventloop.py Outdated Show resolved Hide resolved
@temeddix temeddix changed the title Update and organize examples Improve examples and guides Nov 15, 2023
@temeddix temeddix changed the title Improve examples and guides Improve examples and README Nov 17, 2023
@temeddix
Copy link
Contributor Author

Hi @hosaka , I improved examples, README and things that you have mentioned in #96.

Could you review this PR and merge it if it's okay, before publishing the new version?

@hosaka
Copy link
Collaborator

hosaka commented Nov 17, 2023

Yeah I'm working on it. I have a QML example that I've been meaning to add as well, so I'll update this PR when I put it together.

@hosaka
Copy link
Collaborator

hosaka commented Nov 17, 2023

I've removed a sentence from the readme about GIL and irrelevant Flutter, JavaScript part. I'm assuming that people who want to use qasync have some basic understanding of what async/coroutines are. Let me know if you're happy with the current state of this PR @temeddix

@hosaka hosaka self-assigned this Nov 17, 2023
@hosaka hosaka added the documentation Improvements or additions to documentation label Nov 17, 2023
@hosaka hosaka mentioned this pull request Nov 17, 2023
Closed
@temeddix
Copy link
Contributor Author

temeddix commented Nov 17, 2023 •
edited
Loading

It looks great! I only have one question.

Wouldn't this sentence help people get confident about choosing async over threads? Because for beginners, 'thread' might sound faster than 'async', though it actually isn't.

Single-threaded concurrency does not inherently make the app slower, but only safer, because of Python's GIL.

But I am happy with the current status as well :)

@hosaka
Copy link
Collaborator

hosaka commented Nov 17, 2023

While that sentence does sound encouraging, the readme should describe what the project is and how it should be used to it's full potential, rather than explain abstract concepts :)

@hosaka hosaka merged commit e65c342 into CabbageDevelopment:master Nov 17, 2023
36 checks passed
@temeddix temeddix deleted the update-example branch November 18, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hosaka hosaka hosaka left review comments

Assignees

@hosaka hosaka

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@temeddix @hosaka